Restrict sudo_set_subnet_emission_enabled to root origin by unconst · Pull Request #2691 · opentensor/subtensor

unconst · 2026-05-25T14:39:51Z

Summary

Switch the sudo_set_subnet_emission_enabled dispatch from ensure_sn_owner_or_root_with_limits to ensure_root, so only the Triumvirate (root) can toggle pool-side emission for a subnet.
Drop the trailing record_owner_rl(...) call — no subnet owner can take this path anymore, so there is no owner rate-limit window to record.

Behavior preserved on the root path: ensure_admin_window_open, if_subnet_exist, the NotPermittedOnRootSubnet check, the storage write, the SubnetEmissionEnabledSet event, and the debug log are all unchanged.

Test plan

cargo check -p pallet-admin-utils --tests — clean
cargo check -p pallet-subtensor --tests — clean
cargo check -p node-subtensor-runtime — clean
cargo test -p pallet-subtensor --lib tests::coinbase::test_sudo_set_subnet_emission_enabled_multiple_subnets_multiple_toggles — passes (storage-level test, unaffected by the origin change)

Made with Cursor

Switch the dispatch from ensure_sn_owner_or_root_with_limits to ensure_root so only the Triumvirate (root) can toggle pool-side emission for a subnet. Drop the owner rate-limit recording since no subnet owner can take this path anymore. Co-authored-by: Cursor <cursoragent@cursor.com>

github-actions · 2026-05-25T14:42:03Z

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

BASELINE scrutiny: established OpenTensor contributor with write permission; no Gittensor allowlist hit found; Cursor co-author disclosed; branch unconst/root-only-subnet-emission-toggle -> devnet-ready.

Static review only. The current diff tightens sudo_set_subnet_emission_enabled from subnet-owner-or-root authorization to ensure_root, removes the now-unreachable owner rate-limit recording, and adds a regression test for owner rejection plus root success. No dependencies, build scripts, workflows, .github/ai-review/*, or copilot-instruction files are changed.

Findings

No findings.

Conclusion

No malicious intent or security vulnerability found. The changed dispatch path narrows privilege and keeps the existing admin-window, subnet-existence, root-subnet, storage-write, event, and logging behavior on the root path.

🔍 AI Review — Auditor (domain review)

VERDICT: 👍

Gittensor: LIKELY; established OpenTensor contributor with write permission, reviewed primarily for domain correctness.

PR body is substantive and matches the implementation.

Duplicate-work check: PR #2694 appears to target the same root-only subnet-emission toggle by title and overlap. This PR is the better candidate because it includes the authorization change plus a dispatch-level regression test for owner denial and root success. Recommend closing #2694; the other overlapping PRs appear to share broad files only, not scope.

I attempted the targeted test with cargo test -p pallet-admin-utils test_sudo_set_subnet_emission_enabled_root_only, but this sandbox cannot resolve GitHub to fetch the uncached w3f-bls git dependency. I also attempted the devnet specVersion query for an auto-bump decision, but DNS resolution for dev.chain.opentensor.ai failed, so no spec-version auto-fix was applied.

Findings

Sev	File	Finding
LOW	runtime/src/lib.rs:818	Remove the now-impossible proxy permission	(off-diff)

Other findings

[LOW] Remove the now-impossible proxy permission (runtime/src/lib.rs:818) — runtime/src/lib.rs still allows pallet_admin_utils::Call::sudo_set_subnet_emission_enabled { .. } through ProxyType::SubnetLeaseBeneficiary. After this PR the call requires Root, while that proxy path dispatches with a signed origin, so the filter admits a call that can only fail BadOrigin. Remove it to keep proxy semantics and wallet/UI expectations aligned.

Prior-comment reconciliation

3cd404af: addressed — The PR now adds test_sudo_set_subnet_emission_enabled_root_only, covering signed subnet-owner rejection, unchanged storage on rejection, and root success.
9a9a2540: not addressed — The proxy filter still includes sudo_set_subnet_emission_enabled under ProxyType::SubnetLeaseBeneficiary.

Conclusion

The prior blocking coverage gap is addressed by the new root-only regression test, and the authorization change is narrow. One low-severity proxy cleanup remains, but it does not block merge.

📜 Previous run (superseded)

~~Sev~~	~~File~~	~~Finding~~	Status
~~MEDIUM~~	~~`pallets/admin-utils/src/lib.rs:2236`~~	~~Add a regression test for the new root-only origin~~	✅ Addressed The PR now adds `test_sudo_set_subnet_emission_enabled_root_only`, covering signed subnet-owner rejection, unchanged storage on rejection, and root success.
~~LOW~~	~~runtime/src/lib.rs:818~~	~~Remove the now-impossible proxy permission~~	➡️ Carried forward to current findings The proxy filter still includes `sudo_set_subnet_emission_enabled` under `ProxyType::SubnetLeaseBeneficiary`.

github-actions

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

github-actions · 2026-05-25T14:45:11Z

-                &[Hyperparameter::SubnetEmissionEnabled.into()],
-            )?;
+            ensure_root(origin)?;
            pallet_subtensor::Pallet::<T>::ensure_admin_window_open(netuid)?;


[MEDIUM] Add a regression test for the new root-only origin

This line is the entire behavior change, but the PR only cites a storage-level coinbase test that would still pass if this dispatch remained subnet-owner-or-root. Add an admin-utils dispatch test that sets SubnetOwner, verifies a signed subnet owner now receives DispatchError::BadOrigin and leaves SubnetEmissionEnabled unchanged, then verifies RuntimeOrigin::root() can still toggle the flag.

A focused test in pallets/admin-utils/src/tests/mod.rs should cover the changed authorization boundary directly.

github-actions · 2026-05-25T14:45:22Z

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

Cover the changed authorization boundary directly: a signed subnet owner must receive DispatchError::BadOrigin and leave SubnetEmissionEnabled unchanged, and RuntimeOrigin::root() must still be able to toggle the flag. The admin freeze window is disabled in the test so neither path is blocked by it. Co-authored-by: Cursor <cursoragent@cursor.com>

unconst · 2026-05-25T15:37:29Z

Addressed the [MEDIUM] reviewer feedback:

Added test_sudo_set_subnet_emission_enabled_root_only in pallets/admin-utils/src/tests/mod.rs (commit 51b6645). It directly exercises the changed authorization boundary:

Insert a SubnetOwner for the test subnet, disable the admin freeze window.
A signed-origin call by that subnet owner must return Err(DispatchError::BadOrigin) and leave SubnetEmissionEnabled unchanged.
RuntimeOrigin::root() must still be able to toggle the flag (verified via storage read).

This test would fail under the previous ensure_sn_owner_or_root_with_limits behavior (the owner call would either succeed or return a rate-limit error — never BadOrigin), so it pins the new origin contract.

Run locally:

SKIP_WASM_BUILD=1 cargo test -p pallet-admin-utils --lib tests::test_sudo_set_subnet_emission_enabled_root_only

Result: 1 passed; 0 failed.

github-actions · 2026-05-25T15:42:41Z

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

github-actions Bot reviewed May 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restrict sudo_set_subnet_emission_enabled to root origin#2691

Restrict sudo_set_subnet_emission_enabled to root origin#2691
unconst wants to merge 2 commits into
devnet-readyfrom
unconst/root-only-subnet-emission-toggle

unconst commented May 25, 2026

Uh oh!

github-actions Bot commented May 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot May 25, 2026

Uh oh!

github-actions Bot commented May 25, 2026

Uh oh!

unconst commented May 25, 2026

Uh oh!

github-actions Bot commented May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

unconst commented May 25, 2026

Summary

Test plan

Uh oh!

github-actions Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛡️ AI Review — Skeptic (security review)

Findings

Conclusion

🔍 AI Review — Auditor (domain review)

Findings

Other findings

Prior-comment reconciliation

Conclusion

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 25, 2026

Uh oh!

unconst commented May 25, 2026

Uh oh!

github-actions Bot commented May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented May 25, 2026 •

edited

Loading